Install under ~/.config/fish/conf.d for Fish setup#28
Conversation
For `pnpm setup`, append to ~/.config/fish/conf.d/pnpm.fish instead of ~/.config/fish/config.fish.
📝 WalkthroughWalkthroughThis PR changes the Fish shell integration to write PNPM environment configuration to a dedicated file in ChangesFish shell configuration isolation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
os/env/path-extender-posix/path-extender-posix.spec.ts (1)
986-986: ⚡ Quick winUse
configFile-derived directory instead of cwd-relative mkdir.Line 986 and the other fish setup lines create
'.config/fish/conf.d'relative to the process cwd, while writes target an absoluteconfigFileunderhomeDir. Usingpath.dirname(configFile)makes these tests robust and self-contained.Suggested patch
- fs.mkdirSync('.config/fish/conf.d', { recursive: true }) + fs.mkdirSync(path.dirname(configFile), { recursive: true })Apply the same replacement at Line 1014, Line 1043, Line 1068, Line 1122, Line 1158, and Line 1172.
Also applies to: 1014-1014, 1043-1043, 1068-1068, 1122-1122, 1158-1158, 1172-1172
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@os/env/path-extender-posix/path-extender-posix.spec.ts` at line 986, The tests create the fish config directory relative to cwd using fs.mkdirSync('.config/fish/conf.d', { recursive: true }) but later write to an absolute configFile under homeDir; update each fish-setup mkdir to use the directory derived from configFile (i.e., use path.dirname(configFile)) instead of the hardcoded cwd-relative path so the test directories are created alongside configFile; locate occurrences of fs.mkdirSync that target '.config/fish/conf.d' (and other fish setup mkdirs) and replace them to create path.dirname(configFile) with recursive:true, keeping the same fs.mkdirSync call and options.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@os/env/path-extender-posix/path-extender-posix.spec.ts`:
- Line 986: The tests create the fish config directory relative to cwd using
fs.mkdirSync('.config/fish/conf.d', { recursive: true }) but later write to an
absolute configFile under homeDir; update each fish-setup mkdir to use the
directory derived from configFile (i.e., use path.dirname(configFile)) instead
of the hardcoded cwd-relative path so the test directories are created alongside
configFile; locate occurrences of fs.mkdirSync that target '.config/fish/conf.d'
(and other fish setup mkdirs) and replace them to create
path.dirname(configFile) with recursive:true, keeping the same fs.mkdirSync call
and options.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 756ab793-bb82-40d3-b03a-129a1a9da3df
📒 Files selected for processing (2)
os/env/path-extender-posix/path-extender-posix.spec.tsos/env/path-extender-posix/path-extender-posix.ts
📜 Review details
🔇 Additional comments (2)
os/env/path-extender-posix/path-extender-posix.ts (1)
142-142: LGTM!os/env/path-extender-posix/path-extender-posix.spec.ts (1)
983-983: LGTM!
For
pnpm setup, append to ~/.config/fish/conf.d/pnpm.fish instead of~/.config/fish/config.fish.
Closes pnpm/pnpm#11852.
Summary by CodeRabbit